Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ vectorized feature function support #58

Merged
merged 9 commits into from
Mar 23, 2022
Merged

✨ vectorized feature function support #58

merged 9 commits into from
Mar 23, 2022

Conversation

jvdd
Copy link
Member

@jvdd jvdd commented Mar 19, 2022 •
edited
Loading

TODOs:

  • Update docs with new arguments & explain vectorized feature function behavior
  • Benchmark runtime of vectorized functions
  • Benchmark memory peak of vectorized functions
  • Add proper checks, with appropriate error msg, for equally sampled data assumption
  • Add tests

@codecov-commenter
Copy link

codecov-commenter commented Mar 19, 2022 •
edited
Loading

Codecov Report

Merging #58 (9eb7b06) into main (b983d06) will decrease coverage by 1.75%.
The diff coverage is 58.53%.

@@            Coverage Diff             @@
##             main      #58      +/-   ##
==========================================
- Coverage   98.03%   96.28%   -1.76%     
==========================================
  Files          23       23              
  Lines        1070     1103      +33     
==========================================
+ Hits         1049     1062      +13     
- Misses         21       41      +20
Impacted Files Coverage Δ
tsflex/features/segmenter/strided_rolling.py 88.39% <37.03%> (-9.03%) ⬇️
tsflex/features/function_wrapper.py 100.00% <100.00%> (ø)
tsflex/features/utils.py 100.00% <100.00%> (ø)
tsflex/processing/utils.py 74.19% <0.00%> (-6.46%) ⬇️
tsflex/features/feature_collection.py 99.37% <0.00%> (-0.63%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b983d06...9eb7b06. Read the comment docs.

@jvdd
Copy link
Member Author

jvdd commented Mar 19, 2022

Runtime looks promising though (when there are plenty windows, i.e., small stride)
image

@jvdd
Copy link
Member Author

jvdd commented Mar 19, 2022

This PR opens the path to supporting functions that require to see all the data in a fixed format (nb. windows, window-size); e.g. sklearn-like transforms (see #56 )

@jvdd jvdd requested a review from jonasvdd March 20, 2022 10:41
@jvdd
Copy link
Member Author

jvdd commented Mar 21, 2022 •
edited
Loading

Benchmarking ✔️ (see predict-idlab/tsflex-benchmarking#4)

image

@jvdd
Copy link
Member Author

jvdd commented Mar 21, 2022

@jonasvdd ready for review! 😄

@jonasvdd
Copy link
Member

Maybe also update the poetry lock? (to fix dependabot alerts)

@jonasvdd
Copy link
Member

jonasvdd commented Mar 22, 2022 •
edited
Loading

LGTM! very nice work 💪🏼

@jvdd
Copy link
Member Author

jvdd commented Mar 22, 2022

Maybe also update the poetry lock? (to fix dependabot alerts)

Other dependencies have not updated this yet (which blocks us to update it)

@jvdd
Copy link
Member Author

jvdd commented Mar 23, 2022

Maybe also update the poetry lock? (to fix dependabot alerts)

Other dependencies have not yet updated their dependency on protobuf (resulting in poetry not updating this automatically)..

I'll merge the PR as all other TODOs are ✔️ and leave this open for a future PR!

Thx for the review 😄

@jvdd jvdd merged commit 4d32647 into main Mar 23, 2022
@jvdd jvdd deleted the vectorized_features branch June 19, 2022 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonasvdd jonasvdd jonasvdd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jvdd @codecov-commenter @jonasvdd